Travis fails due to latest libtorrent changes #5263.

Now qBittorrent correctly deletes the root folder if the torrent was added via the magnet link and the metadata was received later.

I don't think i understand what this fixes because i can't even reproduce without this patch.
Add new magnet, uncheck Keep-top level folder and accept before metadata is retrieved? Doesn't create a root folder for me, even when metadata are retrieved later.

Add new magnet, uncheck Keep-top level folder and accept before metadata is retrieved? Doesn't create a root folder for me, even when metadata are retrieved later.

Do you use .!qb extension?

Do you use .!qb extension?

No but i just tried it and couldn't reproduce. I can however reproduce with temp folder. i'll try the patch now.

Still creates root in the temp folder.

Still creates root in the temp folder.

This is not the case. qBittorrent unconditionally creates a subfolder inside the temporary folder for multifile torrents to reduce filename conflict possibility.

Add new magnet, uncheck Keep-top level folder and accept before metadata is retrieved? Doesn't create a root folder for me, even when metadata are retrieved later.

Interesting. Are you sure torrent is added before the metadata is received? Is it in "download metadata" state?
If so, it looks like there are some factors still unknown to me.

Interesting. Are you sure torrent is added before the metadata is received? Is it in "download metadata" state?

Yes.

qBittorrent unconditionally creates a subfolder inside the temporary folder for multifile torrents to reduce filename conflict possibility.

Silly me.. you've said it before but i keep forgetting.

Maybe off topic but the root folder is not deleted from temp folder when delete torrent+files on a torrent with unchecked Keep top-level folder, only the files inside it.

@thalieht, can you perform the following test (using current master/release qBittorrent)?

1. Download some multifile torrent with "Keep top level folder" enabled
2. Store magnet link for this torrent and remove it from the session (don't delete files)
3. Add it again using magnet link into the same save folder but with "Keep top level folder" disabled (make sure it is added before metadata is loaded)

Does it affect previously downloaded data?

Does it affect previously downloaded data?

No, current master.

Interesting case... I usually have to look for an answer to the question why it doesn't work as expected. Now the question is why it works correctly (more precisely, why it gives results as if it works correctly), when it should not.
Well, it seems to be a matter of inter-thread interaction collisions. I did some more tests. Interestingly, master is almost guaranteed to work incorrectly (as it should) when running under the debugger (which affects the speed of thread execution). And I rarely get the same results on a normal run. The fixed version works stably regardless of whether it is run under the debugger or without it.
P.S. I only tested with "Append .!qb extension" option enabled.

@thalieht, can you test how it affects #12604?

@thalieht, can you test how it affects #12604?

Without .!qB it fixes that issue with and without subfolder (didn't test master).
It does not fix it with .!qB enabled. I also didn't test with temp folder.

/offtopic
It was hard testing with .!qB because that option is quite buggy. Enabling/disabling it from the options doesn't add/remove it on existing, half-downloaded files and also it doesn't remove it from all affected files unconditionally, when i delete torrent but keep files

@thalieht, can you test how it affects #12604?

Actually i can't reproduce #12604 in 4.3.0.1 nor in master...

Enabling/disabling it from the options doesn't add/remove it on existing

They should be renamed. If it's not, it's definitely a bug.

it doesn't remove it from all affected files unconditionally, when i delete torrent but keep files

Yes. It's current behavior.
Do you think files should be renamed back in this case? Then you can open another Issue for it.

/Off topic

it doesn't remove it from all affected files unconditionally, when i delete torrent but keep files

Yes. It's current behavior.
Do you think files should be renamed back in this case?

I don't use this feature and since this is known and deliberate, i don't think i should have an opinion on this.
Now that i've given it some thought i can imagine why someone might want to keep .!qB so it seems it's a subjective opinion.

@glassez

Now qBittorrent correctly deletes the root folder if the torrent was added via the magnet link and the metadata was received later.
The existing handling is absolutely incorrect. It was based on false premises. I wonder why it took so long for this to be discovered. Although, apparently, there were related Issues, but most likely left without attention due to incorrect explanations. And I almost never use this feature myself.

What exactly is the original problem(s) that this attempts to solve? Can you please provide more context?

I don't use this feature

Me too.

Now that i've given it some thought i can imagine why someone might want to keep .!qB so it seems it's a subjective opinion.

Apparently for the same purpose - to see incomplete files outside the qBittorrent app. The question is, what is the point of deleting incomplete torrents and saving files? But it seems that someone has a reason to do so.

What exactly is the original problem(s) that this attempts to solve? Can you please provide more context?

When torrent is adding to qBittorrent we apply some preprocessing on its metadata:

1. Remove root folder (if such an option is set)
2. Find existing files on disk in target folder (including possibly incomplete ones) and adjust file names accordingly

There is no problem to apply this preprocessing when torrent is added with metadata provided.
But there is a real problem to do it in case of magnet link since we know nothing about torrent content at this point. The only place where it would seem we could apply it is when the metadata is received. But this should have the same effect as when adding a torrent with metadata, i.e. it should be applied before the torrent starts working (for example, if you choose not to have a root folder, it should not affect possibly existing files in matched folder on disk). There are a number of problems here since libtorrent does not allow to preprocess received metadata before it is used:

1. We can rename all the required files via the lt::torrent_handle interface, but it will not allow you to get rid of the root folder without affecting the corresponding folder on the disk.
2. We could change received metadata but it isn't supposed to be changed (libtorrent provide it as read only). We still can change it but it's kind of a hack.

The Par. 2 is what we're doing now. But, as it turned out, the current logic is incorrect. libtorrent does not use the metadata object itself to handle files, but only to construct file storage and then keep track of the current file layout. Once storage is constructed (based on metadata), it uses its own copy of the file layout structure. We change the metadata in the handler of the metadata_received_alert, which happens outside of libtorrent thread, which means after (generally) the storage is created, and the metadata object can no longer directly affect it.
This PR moves metadata changing in the corresponding place of libtorrent session extension to perform it in libtorrent thread (in the place where metadata is received but storage isn't constructed yet).

Enabling/disabling it from the options doesn't add/remove it on existing

They should be renamed. If it's not, it's definitely a bug.

Should be fixed now.

Should be fixed now.

Indeed.

In fact, the problem is even bigger. Since torrent is supposed to behave the same regardless of the way it is added (.torrent file or magnet link), we have to perform other kind of actions here in addition to handling the root folder. I mean searching for completed/incomplete files, as we do when adding torrent using .torrent file. And this is already more expensive operations that will take up the main libtorrent thread for a longer time, which is not very good.
Maybe of course @arvidn will be able to offer some solution from libtorrent side that allows us to preprocess received metadata and then pass it to libtorrent back (which I strongly doubt). But even if he wants to do this, it will most likely be ABI incompatible, so we will have to wait for it until the next major update (2.1, since 2.0 is already released).
Another solution from the qBittorrent side that I'm thinking of is to delete the torrent after receiving the metadata and then add it again using that metadata. But without a significant reworking of the code, which adds support for such manipulations behind the scenes, it will affect the UI (the torrent will disappear, and then reappear, and it will lose its position in the queue).
From the above, it follows that a good solution will at least take a (more or less) long time to implement, so in any case, we will need some workaround, one of twothree:

1. the approach that is implemented in this PR
2. deleting/adding a torrent in front of the user
3. don't use any workaround and just let it behave differently in case of magnet link

What do you guys prefer?
Or maybe someone has other ideas?

I think the only (or at least the most reliable) way to rewrite the files before adding a torrent, and without affecting files on disk, is to:

• stop the torrent once metadata is received
• grab the metadata
• remove the torrent
• rewrite metadata
• add the updated metadata back

@qbittorrent/frequent-contributors, @qbittorrent/bug-handlers, please participate.

Another solution from the qBittorrent side that I'm thinking of is to delete the torrent after receiving the metadata and then add it again using that metadata. But without a significant reworking of the code, which adds support for such manipulations behind the scenes, it will affect the UI (the torrent will disappear, and then reappear, and it will lose its position in the queue).

I actually like this option. It can be useful not only in this issue. But it's hard for me to predict how long it will take for its implementation, so I would still use some kind of workaround until then.

It will also disconnect peers. To mitigate disruption I would suggest saving resume data and capture the add_torrent_params object, and use that when adding the torrent back again. It will contain a list of peers, to quickly reconnect to them.

Another solution from the qBittorrent side that I'm thinking of is to delete the torrent after receiving the metadata and then add it again using that metadata. But without a significant reworking of the code, which adds support for such manipulations behind the scenes, it will affect the UI (the torrent will disappear, and then reappear, and it will lose its position in the queue).

I actually like this option. It can be useful not only in this issue. But it's hard for me to predict how long it will take for its implementation, so I would still use some kind of workaround until then.

I don't mind the UI disruption, but the queue position resetting is something we should try to mitigate. Maybe display "N/A" for it when the metadata is not yet available? so that users won't have false hope for it.